feat(linear): mint OAuth tokens via AgentCore Identity Token Vault, and latch revoked credentials (#809, #812) - #831

…base

Rebasing onto main (12c9b63) surfaced three collisions with work that
merged after this branch was authored. None were textual conflicts — git
merged the affected files cleanly and produced a build that failed.

1. `allowTestInvoke: false` on the DELETE integration.
   #854 stripped the API Gateway console test-invoke Lambda permissions to
   reclaim CloudFormation resources under the 500-resource ceiling, and
   added a guard asserting none are emitted. This route predates that
   convention, so it re-introduced one and failed the guard 6x (once per
   compute_type x enableToolGateway variant). The two sibling routes in
   this same construct already pass the option; now all three agree.

2. Attributed-Lambda count 46 -> 47.
   The original a19f16d bumped 45 -> 46 for RemoveWorkspaceFn, but main
   independently reached 46, so `git rebase` dropped the commit as "patch
   contents already upstream" — identical text, different reason. The
   textual change survived; the intent did not. With this branch's extra
   Lambda the correct value is 47.

3. Solution user agent on the new handler's SDK clients.
   linear-remove-workspace.ts constructed `new DynamoDBClient({})` and
   `new SecretsManagerClient({})` directly, which drops the solution user
   agent (#319). Now built through makeDocClient()/makeClient(), matching
   linear-link.ts and linear-webhook.ts.

Also resolved the LINEAR_SETUP_GUIDE.md conflict from #831: kept its new
"Vault-managed workspaces" section and dropped the duplicated webhook/
uninstall sentence, which this branch had already relocated into the
parent "Removing a workspace" section. Starlight mirror regenerated.

Refs #306

Conflicts with #831 (Linear OAuth Token Vault), which touched the same three files.
All three were additive on both sides; resolved as unions.

- `cdk/src/constructs/ecs-agent-cluster.ts` — imports. This branch's model constants
  plus main's `LinearIdentityVault`.

- `cdk/src/stacks/agent.ts` — the lambda-microvm `platform_config` block. Both sides
  add fields, so both are kept, but NOT verbatim: main's side calls
  `haikuInferenceProfileId(bedrockGeoRegion)`, and that helper no longer exists here —
  it was removed when the two duplicate haiku-id paths were collapsed into one. Taking
  main's line as written would not compile. Resolved to
  `inferenceProfileId(bedrockGeoRegion, PLATFORM_DEFAULT_AUX_MODEL_ID)`, which is the
  same value through the surviving helper, and the vault fields are kept unchanged.

- `cli/src/platform-doctor.ts` — three hunks: the `linear-auth-health` import, and the
  `Promise.all` destructuring plus its call list. The last two must stay positionally
  aligned, so both were extended in the same order; verified all ten names now line up
  with their outputs.

Also fixed a `{@link haikuInferenceProfileId}` in `bedrock-models.ts` left dangling by
that same helper removal — the merge is what surfaced it, since main's live call site
was the only other reference.

`yarn.lock` moves with main (+42, `@aws-sdk/client-sns` for #831's revocation alerts).

Checked for semantic drift rather than trusting a clean textual merge: the restored
"creates exactly 21 DynamoDB tables" guard is byte-identical to main's at the same line,
so #831 added no table and the merge did not take a stale side of it.

Suites after merge: 4499 CDK, 948 CLI, 1782 agent — `mise run build` clean.

…ed resolver (aws-samples#306) (aws-samples#681)

* feat(cli): bgagent linear remove-workspace + DELETE route + fail-closed resolver (aws-samples#306)

Add a `bgagent linear remove-workspace <slug>` command that deregisters a
Linear workspace, replacing the manual DDB + Secrets Manager surgery that
removal previously required.

CLI (cli/src/commands/linear.ts): new subcommand mirroring add-workspace /
update-webhook-secret UX — slug validation + a "type the slug to confirm"
prompt (skipped by --yes). Delegates all writes to the backend via a new
DELETE call so DDB/Secrets Manager grants stay on the API role, not on every
CLI user (same pattern as `link`). Flags: --purge, --keep-mappings, --yes.

Backend: new flat handler cdk/src/handlers/linear-remove-workspace.ts behind
DELETE /v1/linear/workspaces/{slug} (route + Lambda wired in
cdk/src/constructs/linear-integration.ts). Cognito-authenticated, admin-only
(caller must match the recorded installed_by_platform_user_id). Default is a
SOFT removal: flip the registry row to status=revoked (audit trail preserved)
and delete the bgagent-linear-oauth-<slug> secret; --purge deletes the row
outright. Secret deletion is idempotent (ResourceNotFoundException swallowed,
other SM errors rethrown). Optional project-mapping cleanup keyed on
linear_workspace_id.

Fail-closed resolver: the OAuth resolver already rejects any non-active
registry status (cdk/src/handlers/shared/linear-oauth-resolver.ts) — so a
revoked workspace can no longer resolve a token or route webhooks the instant
this returns. Added an adversarial test proving a revoked slug is rejected
WITHOUT ever reading the secret, plus an active-control case.

Docs: rewrote the "Removing a workspace" section of the Linear setup guide
(Starlight mirror regenerated) to lead with the command and keep the manual
DDB steps as a fallback.

Security: no secrets logged (slug/workspace_id/booleans only); reuses existing
AWS SDK clients; no new dependencies. SAST clean on new files.

Closes aws-samples#306

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* fix(aws-samples#306): loud+recoverable partial teardown, phase logging, coverage gaps

Review follow-up (self-review agents on PR aws-samples#681):

- Secret-delete failure is no longer masked: a non-idempotent SM error
  (e.g. AccessDenied) after the row is revoked now returns a distinct
  500 SECRET_DELETE_FAILED and writes a durable secret_deletion_failed /
  orphaned_oauth_secret_arn marker on the registry row, so the leaked
  credential is discoverable (a naive retry 404s at the active-scan and
  would never re-attempt the delete). ResourceNotFoundException stays
  idempotent (200).
- Top-level catch + mapping-cleanup loop now log the failing phase +
  workspace id / per-page progress so on-call can locate an orphaned
  secret or half-cleaned mapping table from the request id.
- Tests: SECRET_DELETE_FAILED path + marker write; FilterExpression
  pins the status='active' fail-closed filter to the handler (not the
  mock); paginated mapping cleanup across LastEvaluatedKey; missing
  oauth_secret_arn skip; CLI confirmation-prompt abort/proceed;
  secret_deleted:false CLI output; api-client query-string mapping
  (purge / keep_mappings snake_case).

Relates to aws-samples#306

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* docs(aws-samples#306): clarify remove-workspace Lambda timeout rationale

Comment-only: the 30s timeout is higher than the 3s default the
link/webhook handlers use, not "higher than the other request handlers"
(the webhook processor also uses 30s). Nit from PR aws-samples#681 self-review.

Relates to aws-samples#306

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* fix(aws-samples#306): paginate registry scan (fix 404 on live workspaces) + purge revoke-first + drop dead mapping cleanup

B1 (blocking): the registry lookup used `Limit: 1` on a FILTERED Scan.
DynamoDB applies FilterExpression after evaluating `Limit` items, so a
single-row limit examined one arbitrary row, filtered it out, and 404'd a
live workspace whenever the registry held >1 row (the normal shared-stack
state; guaranteed after the first soft-revoke). Drop `Limit` and paginate to
completion, matching jira-webhook-processor.ts / shared/linear-issue-lookup.ts;
document the small-table assumption. Fix the test double so the registry-scan
router honors Limit/ExclusiveStartKey and applies the filter to the examined
slice (this is why B1 was invisible), and add a two-active-row regression with
the target on the second page (fails before, passes after) plus a
follow-LastEvaluatedKey assertion.

B2 (blocking): mapping cleanup was a provable no-op reported as success —
LinearProjectMappingTable rows carry no workspace id (onboard-project writes
none), so the `linear_workspace_id` filter matched zero rows always while the
CLI printed "✓ 0 project mapping(s) removed". Take the reviewer's cheapest
honest fix: remove the mapping-cleanup path entirely — deleteWorkspaceProjectMappings
+ its call, the `--keep-mappings` flag, the projectMappingTable.grantReadWriteData
grant, the LINEAR_PROJECT_MAPPING_TABLE_NAME env, and the 30s timeout bump
(reverted to 10s matching siblings — N3). CLI output + LINEAR_SETUP_GUIDE no
longer claim mapping cleanup; mappings are removed by project id. Schema
follow-up (record linear_workspace_id at onboard time) to be filed separately.

B3 (blocking): on `--purge`, markSecretDeletionFailed early-returned, so a
failed DeleteSecret after the row was deleted leaked the OAuth secret with no
durable record. Reorder to revoke(Update)→DeleteSecret→delete-row(purge only,
after secret confirmed gone), so the marker always lands and fail-closed holds.
Drop the `purged` special-case; correct the two backwards comments (:249-251,
:283-285). Add a --purge marker regression + an ordering assertion.

N2: pin the previously-vacuous construct tests — resolve RemoveWorkspaceFn via
its unique DeleteSecret role grant, pin the DELETE method to the {slug}
resource, and assert the DeleteSecret grant is bound to that role AND scoped to
bgagent-linear-oauth-*.

Closes aws-samples#306

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* test(aws-samples#306): de-tautologize role-binding assertion + fix pagination/test comments + runbook markers (aws-samples#306)

Addresses the 4 non-blocking approval nits from @isadeks:

1. Role-binding assertion (cdk/test/constructs/linear-integration.test.ts):
   findRemoveWorkspaceFn() now derives the remove-workspace role INDEPENDENTLY
   from the FUNCTION resource (its registry-only Environment signature +
   Role Fn::GetAtt), not from the DeleteSecret policy. The secret-prefix test
   asserts the DeleteSecret grant lands on THAT role, so mis-wiring the grant
   onto another role now fails the test (proven: moving the grant to linkFn
   makes `expect(roleRefs).toContain(role)` fail). No longer a tautology.

2. Pagination comment (cdk/src/constructs/linear-integration.ts): the bounded
   sequence now names the lookup phase (registry lookup → revoke → secret
   delete → optional row purge) and states the lookup scan is the only
   paginating phase, bounded by the registry's tens-of-rows scale.

3. Test prose (cdk/test/handlers/linear-remove-workspace.test.ts): :270 comment
   now says "Page 1: empty (no matching row) + a continuation key" to match
   `Items: []`, and the :262-265 preamble no longer describes stale routeDdb
   Limit behavior. Comment-only; test logic unchanged.

4. Runbook markers (docs/guides/LINEAR_SETUP_GUIDE.md + regenerated Starlight
   mirror): the manual-fallback section now names the durable markers
   (secret_deletion_failed / secret_deletion_error / orphaned_oauth_secret_arn)
   and the SECRET_DELETE_FAILED error code, pointing operators to the
   delete-secret fallback.

Closes aws-samples#306

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* fix(aws-samples#306): reconcile DELETE route with post-aws-samples#854 conventions after rebase

Rebasing onto main (12c9b63) surfaced three collisions with work that
merged after this branch was authored. None were textual conflicts — git
merged the affected files cleanly and produced a build that failed.

1. `allowTestInvoke: false` on the DELETE integration.
   aws-samples#854 stripped the API Gateway console test-invoke Lambda permissions to
   reclaim CloudFormation resources under the 500-resource ceiling, and
   added a guard asserting none are emitted. This route predates that
   convention, so it re-introduced one and failed the guard 6x (once per
   compute_type x enableToolGateway variant). The two sibling routes in
   this same construct already pass the option; now all three agree.

2. Attributed-Lambda count 46 -> 47.
   The original a19f16d bumped 45 -> 46 for RemoveWorkspaceFn, but main
   independently reached 46, so `git rebase` dropped the commit as "patch
   contents already upstream" — identical text, different reason. The
   textual change survived; the intent did not. With this branch's extra
   Lambda the correct value is 47.

3. Solution user agent on the new handler's SDK clients.
   linear-remove-workspace.ts constructed `new DynamoDBClient({})` and
   `new SecretsManagerClient({})` directly, which drops the solution user
   agent (aws-samples#319). Now built through makeDocClient()/makeClient(), matching
   linear-link.ts and linear-webhook.ts.

Also resolved the LINEAR_SETUP_GUIDE.md conflict from aws-samples#831: kept its new
"Vault-managed workspaces" section and dropped the duplicated webhook/
uninstall sentence, which this branch had already relocated into the
parent "Removing a workspace" section. Starlight mirror regenerated.

Refs aws-samples#306

* fix(aws-samples#306): report vault-managed teardown as incomplete + settle the revoke race (aws-samples#681 B1, N1-N7, N10-N12)

Addresses PR aws-samples#681 review feedback on `DELETE /v1/linear/workspaces/{slug}`.

B1 (blocking) — the registry lookup could miss an existing workspace.
`ScanCommand` was issued with `Limit: 1` plus a `FilterExpression` on
`workspace_slug`. DynamoDB applies `Limit` to items *examined*, not items
matched, so a filtered scan can legitimately return an empty `Items` array
together with a `LastEvaluatedKey` while the target row sits a page deeper.
On any table with more than one row the handler therefore 404'd on
workspaces that existed. The scan now pages via `ExclusiveStartKey` until
the row is found or the keyspace is exhausted, capped at `MAX_SCAN_PAGES`
(20) so a pathological table cannot pin the Lambda until timeout — the cap
is a 500, not a silent 404, because "we gave up looking" is not "it is not
there". `ConsistentRead: true` was added so a removal issued straight after
a `linear setup` reads its own write.

The paging fix widens, but does not close, a TOCTOU: two concurrent DELETEs
could both find the same `active` row and both report success. The revoke
`UpdateCommand` now carries `ConditionExpression: '#status = :active'`, so
exactly one caller wins; the loser's `ConditionalCheckFailedException` maps
to 404 `WORKSPACE_NOT_FOUND` and, critically, does *not* proceed to delete
the OAuth secret out from under the winner.

N1/N2 — `secret_deleted: boolean` becomes `secret: 'deleted' | 'absent' |
'not_applicable'`. A boolean conflated two very different outcomes: "there
was a secret and it is gone now" and "there was never a Secrets Manager
secret because this workspace is vault-managed". The latter means teardown
is *not* finished — an AgentCore OAuth2 credential provider survives outside
CloudFormation, still holding the Linear client secret and a live,
self-refreshing grant, and `cdk destroy` will not remove it. The response
now echoes `provider_name` for those rows and the CLI prints the exact
`aws bedrock-agentcore-control delete-oauth2-credential-provider` follow-up.
`not_applicable` is deliberately narrow (`providerName && !oauthSecretArn`):
`bgagent linear setup` writes `oauth_secret_arn` unconditionally, so a vault
row that also carries an ARN really did have a secret and reports `absent`.

N3 — the "removes everything" claims in the CLI prompt and the setup guide
were wrong in the vault case. Both now say what is *not* removed, and the
pre-confirmation prompt warns before the destructive action rather than
only disclosing it afterwards.

N7 — the secret delete falls back to the deterministic
`bgagent-linear-oauth-<slug>` name when the row records no
`oauth_secret_arn`, so a partially-written row does not orphan its secret.
`secretsmanager:DeleteSecret` is granted over that name prefix
(`linear-integration.ts`), and `SecretId` accepts a name or an ARN, so the
by-name call is permitted. The prefix is verified identical in all four of
its co-definitions.

N11/N12 — `revoked_reason` is now `admin_removed`, not
`vault_consent_required`. That distinction is load-bearing:
`vault_consent_required` is the one revoked reason the OAuth resolver
re-probes instead of refusing, so reusing it here would let a later
successful vault probe un-latch a workspace an operator deliberately
removed. The vocabulary lives in `LinearRevocationReason` (exported from
`shared/linear-oauth-resolver.ts` as a **type only**) and the writer
declares its own constant. `import type` is erased before esbuild, so the
removal handler takes no runtime dependency on the resolver — a value
import would pull SNS alerting, the resolver's DDB/Secrets Manager clients
and the token-refresh path into this Lambda's bundle, and would land the
handler in the `agent.test.ts` minting-handler census whose entire value is
that such an import is a test failure rather than a production 401.

N4/N5/N6/N10 — `LINEAR_WORKSPACE_REGISTRY_TABLE_NAME` is read once at module
scope and validated in-handler, so a misconfigured deployment 500s with a
named cause instead of an opaque SDK error; the response body is typed by a
module-local interface applied with `satisfies`; the 404-on-lost-race path
logs a WARN naming `oauth_secret_arn`; the 403 wording no longer implies the
workspace exists.

Not included, per review scope: N8 (409 on duplicate active rows for one
slug — currently first-match-wins) and N9 (collapsing the 403 existence
oracle to 404). Both change API semantics and belong in their own issues
rather than a bugfix PR. N9's wording half is done here.

Tests: 24 in the handler suite (by-name delete, `not_applicable` +
`provider_name` echo, vault-row-with-ARN => `absent`, lost race => 404 with
no secret delete, non-conditional update failure => 500, page cap => 500
after exactly 20 scans, `--purge` delete failure => 500 with the revoke
landed, marker-write failure still surfacing `SECRET_DELETE_FAILED`, and a
missing-table-name case in its own module registry). Full suites green:
cdk 4579/4579 (216 suites), cli 941/941 (63 suites), docs 77 pages,
drift-prevention clean, jira-forge-app 11/11.

The agent pytest step of `//agent:quality` was NOT run: this diff contains
no Python, and that suite writes stray commits when run from a worktree
lacking the aws-samples#856 git-config isolation. `//agent:lint` and `//agent:typecheck`
were run instead, both clean.

Refs aws-samples#306, aws-samples#681.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(aws-samples#306): correct the review-item numbering in 253de07's commit message

Empty commit. No code, test, or docs change — this exists only to correct the
record, because `253de07f`'s message cannot be amended without force-pushing a
shared PR branch.

`253de07f` labels its sections with a B1/N-numbering that does NOT correspond
to review 5181793802 on PR aws-samples#681. Every change it describes is a change that was
actually made, and the descriptions are accurate; only the labels are wrong.
Read the labels below, not the ones in that message.

Root of the confusion: PR aws-samples#681 has had two "B1"s. July's round 4807516158 had
B1 = the `Limit: 1` filtered-Scan bug, fixed back then in `0d006b04`. The
current round 5181793802 has B1 = vault-managed teardown reported as complete.
`253de07f`'s message narrates the already-landed July fix under the current
round's B1 heading, which makes the pagination code read as new work when it
predates the commit.

Authoritative mapping of review 5181793802 to what `253de07f` changed:

  B1  vault-managed teardown reported as complete.
      `secret_deleted: boolean` -> `secret: 'deleted' | 'absent' |
      'not_applicable'`, response echoes `provider_name`, CLI prints the
      `delete-oauth2-credential-provider` follow-up, prompt warns before the
      destructive action, guide updated.
      (253de07's message calls this "N1/N2".)

  N1  `ConsistentRead: true` on the lookup scan + `ConditionExpression:
      '#status = :active'` on the revoke, so the loser of a race 404s and
      never reaches `DeleteSecret`.
      (253de07's message folds this into its "B1" section.)

  N2  pagination-rationale cross-references corrected: dropped
      `jira-webhook-processor.ts`, cited `shared/jira-tenant-registry.ts:32-46`
      as the genuine precedent and as the `ConsistentRead` precedent, kept
      `linear-issue-lookup.ts:131-136` relabelled a counter-example, and
      replaced "paginate to completion" with "until a match or key exhaustion".
      (253de07's message does not label this at all.)

  N3  the `status != 'active'` claim qualified in five places, and the removal
      now writes the distinct `revoked_reason: 'admin_removed'`.
      (253de07's message splits this across its "N3" and "N11/N12".)

  N4  `DeleteSecretCommand` input asserted exactly: `SecretId` plus
      `ForceDeleteWithoutRecovery: true`.
      (253de07's message lists this under "N4/N5/N6/N10" without detail.)

  N5  all three `JSON.stringify(...).toContain('revoked')` assertions replaced
      with value-level `[':revoked'] === 'revoked'` and `[':uid'] === ADMIN`.

  N6  the three untested failure paths covered: revoke rejecting (500, no
      `DeleteSecret`), `markSecretDeletionFailed` rejecting (still
      `SECRET_DELETE_FAILED`), `--purge` `DeleteCommand` rejecting (500 with
      the revoke asserted landed).

  N7  `MAX_SCAN_PAGES = 20`, returning 500 rather than 404.
      (253de07's message uses "N7" for the by-name secret-delete fallback,
      which was not a numbered review item.)

  N8  NOT in this commit. Filed as aws-samples#883 (409 on duplicate active rows).

  N9  wording half only. The behavioural half is filed as aws-samples#884.

  N10 manual-fallback snippet reordered to revoke-first with
      `--condition-expression '#s = :active'`, plus the note that re-running
      `remove-workspace` on an already-removed workspace returns 404.

  N11 module-local `RemoveWorkspaceResponseBody` applied with `satisfies` at
      the return.
      (253de07's message lists this under "N4/N5/N6/N10".)

  N12 `WORKSPACE_REGISTRY_TABLE` is plain `string | undefined`, no non-null
      assertion, validated once in-handler.
      (253de07's message lists this under "N4/N5/N6/N10".)

Also not a numbered review item, and therefore unlabelled rather than
mislabelled: the by-name `bgagent-linear-oauth-<slug>` fallback for the secret
delete when the row records no `oauth_secret_arn`.

The full mapping with reasoning is in the PR comment:
aws-samples#681 (comment)

Refs aws-samples#306, aws-samples#681, aws-samples#883, aws-samples#884.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Sphia Sadek <isadeks@gmail.com>

Verification could not tell a workspace's own signing secret from a copy of
another workspace's, because the only available signal was value equality
against the stack-wide secret — and that signal is wrong. A healthy
single-workspace install also holds a secret equal to the stack-wide copy, since
the first install stamps the same real secret into both slots. Rejecting on
equality would 401 exactly the deployments that are safe.

So ownership is now recorded by provenance instead: `webhook_secret_owned` on the
registry row, written from where the value came from. `setup` sets it when the
operator supplied the secret for this workspace or when a stored `lin_wh_` value
differs from the stack-wide one. `update-webhook-secret` sets it too — that is
the remediation path, and without it an operator could paste the correct secret
and see no change in how the workspace is treated.

Only a literal `true` is read back as proof. Absence means "not proven", which is
what every row predating this field is, and is deliberately not the same as
"proven not owned".

`update-webhook-secret` gains `--stack-name` so it can reach the registry, and
fails loudly if the secret write succeeds but the provenance write does not —
silence there would leave the operator believing a fix landed while the state
verification reads is unchanged.

Two things found while building this, both worth knowing:

`mirror-stackwide` — the branch that copies the first workspace's secret into an
additional workspace — is already unreachable from `setup` on current main. #831
added an up-front prompt that throws when neither a supplied nor a legitimately
stored secret exists, which is the only way into that branch. The refusal added
here is therefore a safety net, not a live fix; it is extracted as
`assertMirrorIsSafe` and unit-tested directly, because a guard reachable only
through dead code is otherwise an untested guard. The remaining exposure is rows
written by older releases and not re-run since.

The provenance derivation cannot currently evaluate false, so mutating it to a
literal `true` leaves every test passing. That is semantic equivalence under the
current preconditions rather than a test gap — there is no reachable input that
distinguishes them. It is kept as a derivation because it becomes load-bearing if
that up-front prompt is ever relaxed, and the resulting failure would be silent.

…ign (aws-samples#879) (aws-samples#880)

* fix(security): merge vault channel_metadata by spread, not Object.assign (aws-samples#879)

`Object.assign` copies via [[Set]], which invokes the `__proto__` setter, so the
pattern sits one refactor away from a prototype-pollution sink. Object spread
defines own properties instead, where the same key lands inert.

Not reachable today — `vaultMetadata` returns a literal with two hard-coded keys —
but semgrep rates javascript.lang.security.insecure-object-assign as Blocking, and
`security:sast` runs whole-repo in the pre-push hook while security-pr.yml runs only
the ranged gates. So this one line passed PR CI on aws-samples#831 and then rejected every
contributor's `git push`, on every branch, for as long as it sat on main.

Spread rather than explicit keyed writes because `vaultMetadata` is the single place
that says which vault fields a task carries; restating them at this call site is the
silent field-drop the helper exists to prevent.

Tests — two source-level guards, failing for different reasons:

* The existing structural check keys off the literal-entry form
  (`linear_workspace_slug: resolved.workspaceSlug,`), so it never fired on this
  builder at all: the one path `vaultMetadata` was written for was the one path its
  own guard never covered. Extended to the assignment form, scoped by dedent because
  an assignment has no literal to terminate.
* A ban on `Object.assign` onto a metadata object. Reverting this change is
  behaviour-preserving, so the existing behavioural test stays green through it — a
  security property that lives in *how* the copy happens needs a structural check.

Mutation-verified: restoring `Object.assign` reds both new guards and neither the
behavioural test; deleting the merge reds the assignment-form guard and the
behavioural test but not the Object.assign ban.

`mise run build` fails locally at `//cdk:synth:quiet` on an unrelated host IAM gap
(`ec2:DescribeAvailabilityZones` denied to the local role, needed for the AgentCore
AZ context lookup). cdk compile, eslint, and 215 suites / 4542 tests pass.

Closes aws-samples#879

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(cdk): drop the dead workspace-id write and correct the stale guard docstring (aws-samples#880 review)

Three review items on the handler, all non-blocking.

1. Deleted `channelMetadata.linear_workspace_id = workspaceId;`. It was dead, and this
   PR is what killed it: the object literal above already sets that key to the same
   value from the same variable, and `vaultMetadata` does not emit the key, so the
   spread cannot clobber it either. Confirmed by enumerating every writer of the key,
   not by reading the happy path.

2. Corrected `vaultMetadata`'s docstring. It still claimed the source-level check "keys
   off the object-literal form and cannot see a builder that assigns onto an existing
   object" — the premise of this PR, and false since the assignment-form guard landed.
   It now describes three checks: the literal form, the assignment form, and the
   behavioural one.

3. Trimmed the rationale at the call site from 24 comment lines to 13. Kept the two
   load-bearing claims — spread rather than `Object.assign` because [[Set]] invokes the
   `__proto__` setter, and spread the helper so a newly added vault field cannot be
   silently dropped — plus the recorded-subject note. The guard archaeology and the
   pre-push history now live in the commit trail and aws-samples#879, where they cannot drift out
   of sync with the code.

Refs aws-samples#879.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(cdk): close two holes in the vault-metadata source guards (aws-samples#880 review)

Both guards now match code, never prose, via a shared `isCommentLine`, and the
assignment-form guard requires the merge to hit the same object the trigger wrote to.

`assignment-form builders also carry the vault fields`

The review's diagnosis was right: the accept window is far wider than it looks. The
trigger sits at the shallowest indent of `if (WORKSPACE_REGISTRY_TABLE)`, so the scan
ran ~45 lines to the end of that block — a `...vaultMetadata(resolved)` in any later
sibling statement satisfied a builder that merged nothing into its own object.

The suggested remedy is not applied as written, deliberately. Breaking on the first line
indented `<=` the trigger reds the currently passing case on a comment at the same
indent, and more fundamentally the real spread sits one level DEEPER, inside
`if (resolved.providerName)`, because the merge is conditional while the slug write is
not. Same-scope is therefore not an invariant this code can assert; same-object is, and
it closes the same hole. The dedent break is kept as the scope exit and now precedes the
accept check, which is stricter than before — the old order accepted a dedented spread.

`no metadata builder merges via Object.assign`

Regex unqualified over this file. `/Object\.assign\s*\(\s*\w*[Mm]etadata\b/` required the
first argument to be a bare identifier containing "metadata", so `Object.assign(md, …)`
and `Object.assign(row.channelMetadata, …)` reintroduced the sink without redding the
test — `\w*` cannot cross a `.`. Scope stays this one handler on purpose; the
class-level net is the whole-repo semgrep gate that `security-pr.yml` omits, tracked in
aws-samples#235 rather than bolted on here.

Comment-stripping is load-bearing for the vaultMetadata guard, whose `includes()` test
was satisfied by a comment merely mentioning the spread (mutation M3 below). For the
`Object.assign` ban it is defensive rather than required: the handler's three prose
mentions all write the name in backticks, so `\s*` never reaches a `(`. Incidental
punctuation, not a property worth depending on.

Mutation-verified rather than asserted. Each row was applied to the handler, the named
guards observed red, then reverted and the source re-verified by content:

  M1  merge → `Object.assign(channelMetadata, …)`      both guards red
  M2  merge → `Object.assign(md, …)`                   both guards red   (before: BLIND)
  M3  merge deleted, comment still names the spread    assignment guard red (before: BLIND)
  M4  merge moved to a sibling block, other object     assignment guard red (before: BLIND)

M2, M3 and M4 are the differentials — all three passed before this change.

Gates: `//cdk:test` 215 suites / 4552 tests green; this file 61/61; `//cdk:eslint` clean
with no `--fix` mutations; `//cdk:compile` clean; `//cdk:synth:quiet` green once the AZ
context is pinned (it otherwise fails closed on a local role lacking
`ec2:DescribeAvailabilityZones`, as documented in DEPLOYMENT_GUIDE.md — unrelated to
this change).

Note for anyone rerunning: `-t` filtering on this file is unreliable and not a
regression. Two behavioural tests in this describe have no `beforeEach` of their own and
depend on `probeLinearIssueContextMock` being armed by the first describe's, so any
filter that skips it fails them on pristine HEAD too. Run the whole file.

Refs aws-samples#879, aws-samples#235.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Sphia Sadek <isadeks@gmail.com>

… the delivery (aws-samples#917)

* feat(linear): record the owning workspace on project mappings

The Linear project-to-repo mapping table is keyed on `linear_project_id` alone
and carries no workspace attribute, so nothing ties a mapping to the tenant that
owns it. The webhook processor looks the repository up from the body-supplied
`projectId` with no way to check it against the workspace whose signature the
delivery arrived with, which lets any onboarded workspace name another
workspace's project and steer an agent task at that workspace's repository.

This records the missing value so a later change can enforce it; no read path
changes behaviour yet.

`onboard-project` now resolves the owning workspace by asking Linear which
onboarded workspace's own token can see the project, rather than accepting it as
a flag. The resolution is what the enforcement path will check deliveries
against, so a typo'd flag would durably write the cross-tenant mapping the check
exists to prevent — and `organization.id` is read from the same authenticated
response that resolved the project, not from the workspace we assumed we were
asking. `--slug` narrows the search; `--workspace-id` records an owner without
verifying it, for when the Linear API is unreachable.

`backfill-project-workspaces` fills in mappings that predate the field, one pass
per workspace rather than one lookup per row, and paginates the project listing
(`list-projects` stops at 100, which for a backfill would silently leave the
overflow unresolved). The update is conditional on the row still existing and
still having no workspace id, so it neither resurrects a deleted mapping nor
overwrites a concurrent `onboard-project`. A project id claimed by two
workspaces is skipped and named rather than assigned to whichever answered
first.

`platform doctor` reports mappings with no owning workspace, capped at ten named
ids, so an operator can tell whether the backfill is finished before enforcement
is turned on.

* feat(linear): check a project mapping against the workspace that sent the event

The processor resolved the target repository from the body-supplied `projectId`
alone. The mapping table is keyed on that id with no workspace attribute, so a
workspace holding its own valid signing secret could sign an event with its own
`organizationId` while naming another workspace's project, and the processor
would run an agent task on that workspace's repository. Nothing about that
attack needs a shared or stack-wide secret, which is why signature-level fixes
do not reach it.

Now the mapping's `linear_workspace_id` is checked against the event's
`organizationId` and a mismatch drops the delivery.

Dropped rather than answered. The reply would be posted into the sender's own
workspace, and "that project belongs to someone else" both confirms the project
exists and tells a prober the attempt was seen. The log line carries both ids
and is the diagnostic surface instead.

A mapping with no owning workspace recorded still proceeds, and warns. Rejecting
those on deploy would break every install that has not run
`backfill-project-workspaces` yet; turning it into a rejection is a later step,
gated per environment once `platform doctor` reports the backfill clean.

Both rejection tests were confirmed to fail with the check disabled. The
no-workspace-id case originally passed either way — an event with no
`organizationId` never creates a task regardless, because it fails later at
attribution — so it asserts the guard's own log line instead, which is the only
thing that distinguishes a drop here from a drop there.

* feat(linear): record whether a workspace owns its webhook signing secret

Verification could not tell a workspace's own signing secret from a copy of
another workspace's, because the only available signal was value equality
against the stack-wide secret — and that signal is wrong. A healthy
single-workspace install also holds a secret equal to the stack-wide copy, since
the first install stamps the same real secret into both slots. Rejecting on
equality would 401 exactly the deployments that are safe.

So ownership is now recorded by provenance instead: `webhook_secret_owned` on the
registry row, written from where the value came from. `setup` sets it when the
operator supplied the secret for this workspace or when a stored `lin_wh_` value
differs from the stack-wide one. `update-webhook-secret` sets it too — that is
the remediation path, and without it an operator could paste the correct secret
and see no change in how the workspace is treated.

Only a literal `true` is read back as proof. Absence means "not proven", which is
what every row predating this field is, and is deliberately not the same as
"proven not owned".

`update-webhook-secret` gains `--stack-name` so it can reach the registry, and
fails loudly if the secret write succeeds but the provenance write does not —
silence there would leave the operator believing a fix landed while the state
verification reads is unchanged.

Two things found while building this, both worth knowing:

`mirror-stackwide` — the branch that copies the first workspace's secret into an
additional workspace — is already unreachable from `setup` on current main. aws-samples#831
added an up-front prompt that throws when neither a supplied nor a legitimately
stored secret exists, which is the only way into that branch. The refusal added
here is therefore a safety net, not a live fix; it is extracted as
`assertMirrorIsSafe` and unit-tested directly, because a guard reachable only
through dead code is otherwise an untested guard. The remaining exposure is rows
written by older releases and not re-run since.

The provenance derivation cannot currently evaluate false, so mutating it to a
literal `true` leaves every test passing. That is semantic equivalence under the
current preconditions rather than a test gap — there is no reachable input that
distinguishes them. It is kept as a derivation because it becomes load-bearing if
that up-front prompt is ever relaxed, and the resulting failure would be silent.

* feat(linear): refuse a signing secret that cannot attest which workspace sent a delivery

The processor routes from identifiers in the request body, so verification has to
establish WHICH tenant sent a delivery — not merely that the sender knows a secret
this stack accepts. Two secrets fail that test: the stack-wide one, which is bound
to no workspace, and a per-workspace one that is a copy of another workspace's,
which older releases could install.

Both are now refused, and both only where they can actually cross a boundary:

- `verifyLinearRequestForWorkspace` gains a `shared-secret` outcome. A matching
  signature on a row that does not record the secret as its own is rejected when
  another workspace is active. Fatal like `mismatch`, for the same reason —
  falling through would re-admit it via the stack-wide path.
- The receiver skips the stack-wide fallback entirely on a multi-workspace stack.
  This is what supersedes sole-tenant binding rather than implementing it: with
  the fallback gone under multi-tenancy, that path survives only where there is
  one tenant, and there it cannot name the wrong one.
- `verified_via_stack_wide` is forwarded to the processor, which replaces the
  body's `organizationId` with the sole active workspace and drops the delivery
  when there is no single answer. The field is overwritten on the payload rather
  than threaded as a parameter because the workspace id is read from about six
  places downstream; passing it alongside would leave every one of them a site
  where the claimed value could still be picked up.

Single-workspace installs are untouched by design. Every check consults the active
workspace count first, and with one tenant a tenant-less secret still identifies
the only tenant there is. That also makes an absent provenance flag — the state of
every row written before it existed — harmless on the installs where it is normal.

`countActiveLinearWorkspaces` saturates at 2, since no caller needs the true
total, and answers 1 when the registry cannot be read. That is deliberately the
permissive direction: these checks harden a signature that has already verified,
so a DynamoDB throttle must not become a stack-wide outage.

No IAM change. `grantReadData` already covers `dynamodb:Scan` for both the
receiver and the processor on the registry table.

All three guards were confirmed to fail with the check disabled. One test needed
fixing first: the multi-workspace fallback case passed because the harness shrank
the workspace count along with the absent registry row, so the receiver had only
ever seen one workspace.

* feat(linear): record provenance for secrets that were never mirrored

Enforcing per-workspace secret ownership needs the provenance flag to already be
set, and it is set only by `setup` and `update-webhook-secret`. On an existing
multi-workspace stack no row has it, so turning enforcement on rejects every
workspace at once — including workspaces that genuinely own their secret and are
being broken for nothing. On the dev stack that is 3 of 3.

`backfill-secret-provenance` closes the gap for the workspaces where ownership can
actually be established. `mirror-stackwide` can copy nothing except the stack-wide
value, so a stored secret that DIFFERS from it was never mirrored and is therefore
owned. That conclusion cannot be wrong, which is what makes it safe to write.

The converse is deliberately not used. A secret EQUAL to the stack-wide value is
equally consistent with a healthy single-workspace install, whose first `setup`
stamps the same real secret into both slots — so `inherited` withholds a verdict
rather than asserting "not owned", and only `own` is recorded. This is the same
asymmetry the earlier commit's doc comment warns about, now expressed as code: the
direction that grants is sound, the direction that would reject is not.

Reading the stack-wide secret is fatal on failure rather than treated as "not set".
Continuing with undefined would classify every workspace as owning its secret,
since nothing can equal a value that was never fetched — recording exactly the
shared secrets this command exists to withhold.

`platform doctor` reports the same population, gated on the active workspace count
so it matches the enforcement path: on a single-workspace stack an unrecorded
provenance is the normal state for every pre-existing row and carries no risk, so
it passes with a note rather than warning.

Both directions of the classifier were confirmed to fail when mutated — treating
`equals stack-wide` as owned, and treating an empty stored secret as present.

* fix(linear): do not blame stale mappings for a workspace we could not query

The first live run of `backfill-project-workspaces` hit two workspaces whose
Linear grants had expired. Both returned 401, so every mapping they own came back
unresolved — and the report concluded those mappings "name projects no onboarded
workspace can see" and told the operator to re-onboard or remove them. Four valid
rows, and the suggested remedy was to delete them.

An unresolved mapping means opposite things depending on whether every workspace
was actually reachable, so the conclusion now branches on that. Skipped workspaces
are tracked wherever the loop gives up on one — no token, an API error, or a
response with no organization id — and when any exist the report leads with "Do
NOT delete these yet", names them, and points at restoring their access first.
The delete-them wording now appears only when every workspace was queried
successfully.

Verified by re-running against the dev stack, where the output now names
demo-abca and maguireb as the reason rather than implicating their projects.

The guard is asserted at source level: the branch sits inside a Commander action
that would need the whole AWS and Linear surface stood up to reach, and the
behavioural proof is the live run. The test pins that the unconditional wording
cannot come back.

* fix(linear): refuse to half-repair a workspace when the registry cannot be found

`update-webhook-secret` writes two things: the signing secret, and the registry
flag recording that the workspace owns it. Verification reads the flag, so without
it the repair changes nothing observable — the workspace keeps 401ing.

The registry write was wrapped in `if (registryTableName)`, so an operator who
defaulted or mistyped `--stack-name` got the secret written, no flag, and a green
"✅ Updated webhook signing secret". That is precisely the silent-miss the comment
directly above the block claims to prevent, and it would send someone chasing a
401 whose cause was in the output they already saw.

Now it refuses, names the stack it looked in, says the secret was preserved, and
points at `--stack-name`.

* refactor(linear): drop migration tooling for a state one deployment is in

The fix itself is 257 lines of cdk/src. The CLI around it had grown to 926 —
mostly tooling to migrate stacks into the new model. Reviewed against how many
deployments actually need migrating, two pieces did not earn their weight.

`backfill-project-workspaces` is gone, with its paginated project lister. A
single-workspace install does not need it at all: with one tenant every mapped
project belongs to that tenant, so a body-supplied projectId cannot steer a task
across a boundary. Its real audience was "a stack with two or more workspaces AND
pre-existing mappings", which is one deployment today. It was also the only part
of the branch that needed live Linear tokens, and the only part that has already
produced a defect — a report that told an operator to delete four valid mappings
because two workspaces had expired grants.

The doctor check that reported those mappings stays, because knowing about them
is still worth an operator's attention. Its remedy is now "re-run onboard-project
before you add a second workspace", and it says plainly that the state is harmless
while one workspace is active.

`assertMirrorIsSafe` is gone too. It guarded a branch that is unreachable from
`setup`, and the value that branch would produce is no longer trusted on its own
anyway — the receiver rejects a secret not recorded as the workspace's own once a
second workspace is active. The comment there now describes what the code does
rather than the refusal that used to follow it, and says what would need to change
if the up-front prompt is ever relaxed.

`backfill-secret-provenance` stays. This is the one place doing it by hand is
actively dangerous rather than merely tedious: the safe rule holds in one
direction only, and setting the flag on a workspace that holds the shared secret
silently reopens the hole. It also runs with no Linear dependency at all, which is
why it is the piece that already works on a stack with two dead grants.

935 tests green. The classifier's mutation was re-run after the removal, since it
sat between two deleted blocks and had to be restored.

* test(linear): note why the fake UUIDs avoid twelve trailing digits

The org-id fixtures use hex letters in the final group because a UUID ending in
twelve consecutive digits matches gitleaks' `aws-account-id` rule, which failed
CI's range-scoped secret scan on values that hold no secret.

The values themselves were corrected in place across this branch's history —
fixing them in a follow-up commit left them in the commit that introduced the
file, and the scan is range-scoped over every commit in the PR. Recorded as a
comment so the constraint survives the next person editing these constants.

Fixed in the fixture rather than allowlisted in `.gitleaks.toml`: widening a
shared security rule to accommodate a test constant is the wrong direction.

* feat(linear): scope @bgagent comment targets to the sending workspace

Comment triggers find their target by issue id, through the orchestration
or the newest task on the issue. Match that target's recorded workspace
(the orchestration's credentials_ref, or the task's
channel_metadata.linear_workspace_id) against the workspace the delivery
came from, and act only when they match. This brings the comment path in
line with the project-mapping workspace check added earlier on this
branch.

The task lookup now returns linear_workspace_id from channel_metadata,
which the LinearIssueIndex projection already includes. Test fixtures
now use a single workspace throughout.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(linear): end-to-end coverage for webhook workspace binding

Runs a delivery through the real receiver and hands the receiver's
processor invoke to the real processor, which runs the real token
resolver, user lookup, orchestration store and createTaskCore. All
tables and secrets live in one stateful fake shared by both handlers, so
the receiver's workspace count and the processor's binding read the same
registry. Only the modules that call Linear's API are mocked, and network
access is refused.

Each case asserts whether a task record was persisted, and for which
repository, platform user and workspace. Covered:

- several workspaces: signature and workspace rules for owned,
  stack-wide-only, shared and unregistered secrets, a tampered body,
  a revoked workspace, project ownership, and actor scoping
- one workspace: the stack-wide fallback binds to the only workspace;
  lingering mappings from inactive workspaces are not served; a second
  workspace onboarded inside the receiver's count-cache window is caught
  by the processor
- @bgagent comments on epics, sub-issues, plain issues and retries act
  only on the sending workspace's work, with matching-workspace controls

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(cli): make doctor's Linear verdicts match the workspace count and admin removals

Found while live-testing this branch on a multi-workspace stack:

- An admin-removed workspace (`remove-workspace`, revoked_reason=admin_removed)
  was reported as a REVOKED authorization: doctor failed and told the operator
  to re-authorize a workspace they had removed on purpose. It is now `disabled`,
  the state that already exists for intentional drops, and the pass line no
  longer counts it as authorized.
- The unowned-project-mapping check said "harmless while one workspace is
  active" whatever the count. With more than one active workspace the webhook
  path admits an unowned mapping with only a warning, so the check now reads
  the registry: it fails above one workspace, keeps the "harmless" wording for
  exactly one, and makes no claim when the registry cannot be read.
- A processor comment named `backfill-project-workspaces`, which this branch
  removed; it now names `onboard-project`.

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: bgagent <bgagent@noreply.github.com>